-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update generator to use Griffel #21390
chore: update generator to use Griffel #21390
Conversation
@@ -16,7 +16,7 @@ const v8ReferencePackages = { | |||
node: ['codemods'], | |||
}; | |||
const convergedReferencePackages = { | |||
react: ['react-menu'], | |||
react: ['react-provider'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently only @fluentui/react-provider
is using Griffel packages (#21360). As this script pickups versions from a package.json
I switched it to use different package as a source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh didnt know we have thing like this, nx migration is run after the plop anyways, so it should be always up to date to latest setup
"@fluentui/babel-make-styles": "", | ||
"@fluentui/eslint-plugin": "", | ||
"@fluentui/jest-serializer-make-styles": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fluentui/babel-make-styles
& @fluentui/jest-serializer-make-styles
are defined on root package.json
as now they are third party devDependencies
.
@@ -326,7 +326,6 @@ describe('migrate-converged-pkg generator', () => { | |||
expect(rootTsConfig.compilerOptions.paths).toEqual( | |||
expect.objectContaining({ | |||
'@proj/react-dummy': ['packages/react-dummy/src/index.ts'], | |||
'@proj/react-make-styles': ['packages/react-make-styles/src/index.ts'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package will be removed soon.
}); | ||
expect(devDeps[babelMakeStylesPkg]).toBe('9.0.0-alpha.0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to have Babel plugin in devDependencies
of the package, that's why tests were trimmed a lot 🐱
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a8551c9:
|
} | ||
const shouldAddGriffelPreset = pkgJson.dependencies['@griffel/react']; | ||
const extraPresets = shouldAddGriffelPreset ? ['@griffel'] : []; | ||
const config = templates.babelConfig({ extraPresets }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to add Babel plugin to devDependencies
, this simplifies this part a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me gusta
📊 Bundle size reportUnchanged fixtures
|
@@ -34,6 +34,7 @@ | |||
"@types/enzyme-adapter-react-16": "1.0.3", | |||
"@types/react": "16.9.42", | |||
"@types/react-dom": "16.9.10", | |||
"@types/react-test-renderer": "^16.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fluentui/scripts/create-package/plopfile.ts
Lines 166 to 170 in 7063053
function replaceVersionsFromReference( | |
referencePackages: string[], | |
newPackageJson: PackageJson, | |
answers: Answers, | |
): void { |
The script used for create-package
reads & uses versions of dependencies, if this dependency absent it fails:
Opened for better suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we hoist this dev dependency ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to move it to root package.json
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to move all of these devDep things to single version policy, would be nice to implement this in separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm working on PR to hoist this to single version policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 706305303fcf03a82052c0a9e74d8aecb102885e (build) |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
ContextualMenu | mount | 16471 | 7534 | 1000 | Possible regression |
All results
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 786 | 763 | 5000 | |
BaseButton | mount | 809 | 832 | 5000 | |
Breadcrumb | mount | 2364 | 2303 | 1000 | |
ButtonNext | mount | 470 | 454 | 5000 | |
Checkbox | mount | 1367 | 1366 | 5000 | |
CheckboxBase | mount | 1137 | 1147 | 5000 | |
ChoiceGroup | mount | 4172 | 4158 | 5000 | |
ComboBox | mount | 866 | 876 | 1000 | |
CommandBar | mount | 9102 | 9142 | 1000 | |
ContextualMenu | mount | 16471 | 7534 | 1000 | Possible regression |
DefaultButton | mount | 1002 | 1015 | 5000 | |
DetailsRow | mount | 3329 | 3308 | 5000 | |
DetailsRowFast | mount | 3329 | 3311 | 5000 | |
DetailsRowNoStyles | mount | 3163 | 3156 | 5000 | |
Dialog | mount | 2318 | 2290 | 1000 | |
DocumentCardTitle | mount | 180 | 154 | 1000 | |
Dropdown | mount | 2833 | 2856 | 5000 | |
FluentProviderNext | mount | 1690 | 1743 | 5000 | |
FluentProviderWithTheme | mount | 141 | 143 | 10 | |
FluentProviderWithTheme | virtual-rerender | 114 | 113 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 178 | 205 | 10 | |
FocusTrapZone | mount | 1613 | 1661 | 5000 | |
FocusZone | mount | 1612 | 1666 | 5000 | |
IconButton | mount | 1546 | 1537 | 5000 | |
Label | mount | 331 | 338 | 5000 | |
Layer | mount | 2646 | 2668 | 5000 | |
Link | mount | 431 | 429 | 5000 | |
MakeStyles | mount | 1496 | 1495 | 50000 | |
MenuButton | mount | 1294 | 1282 | 5000 | |
MessageBar | mount | 1779 | 1779 | 5000 | |
Nav | mount | 2940 | 2943 | 1000 | |
OverflowSet | mount | 993 | 984 | 5000 | |
Panel | mount | 2191 | 2208 | 1000 | |
Persona | mount | 770 | 758 | 1000 | |
Pivot | mount | 1276 | 1293 | 1000 | |
PrimaryButton | mount | 1148 | 1143 | 5000 | |
Rating | mount | 6819 | 6698 | 5000 | |
SearchBox | mount | 1201 | 1194 | 5000 | |
Shimmer | mount | 2241 | 2224 | 5000 | |
Slider | mount | 1713 | 1732 | 5000 | |
SpinButton | mount | 4502 | 4416 | 5000 | |
Spinner | mount | 407 | 399 | 5000 | |
SplitButton | mount | 2764 | 2771 | 5000 | |
Stack | mount | 466 | 484 | 5000 | |
StackWithIntrinsicChildren | mount | 1982 | 1994 | 5000 | |
StackWithTextChildren | mount | 4549 | 4554 | 5000 | |
SwatchColorPicker | mount | 10068 | 10090 | 5000 | |
TagPicker | mount | 2308 | 2342 | 5000 | |
TeachingBubble | mount | 11714 | 11624 | 5000 | |
Text | mount | 391 | 397 | 5000 | |
TextField | mount | 1236 | 1206 | 5000 | |
ThemeProvider | mount | 1066 | 1061 | 5000 | |
ThemeProvider | virtual-rerender | 554 | 549 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 1679 | 1652 | 5000 | |
Toggle | mount | 723 | 742 | 5000 | |
buttonNative | mount | 137 | 142 | 5000 |
Perf Analysis (@fluentui/react-northstar
)
Perf tests with no regressions
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
ButtonMinimalPerf.default | 168 | 148 | 1.14:1 |
AvatarMinimalPerf.default | 184 | 170 | 1.08:1 |
AttachmentMinimalPerf.default | 146 | 138 | 1.06:1 |
BoxMinimalPerf.default | 314 | 297 | 1.06:1 |
FormMinimalPerf.default | 370 | 349 | 1.06:1 |
ReactionMinimalPerf.default | 346 | 326 | 1.06:1 |
SegmentMinimalPerf.default | 322 | 305 | 1.06:1 |
ChatWithPopoverPerf.default | 331 | 316 | 1.05:1 |
FlexMinimalPerf.default | 259 | 249 | 1.04:1 |
HeaderSlotsPerf.default | 673 | 647 | 1.04:1 |
AlertMinimalPerf.default | 242 | 234 | 1.03:1 |
ButtonSlotsPerf.default | 478 | 463 | 1.03:1 |
CardMinimalPerf.default | 490 | 474 | 1.03:1 |
ChatDuplicateMessagesPerf.default | 265 | 258 | 1.03:1 |
LabelMinimalPerf.default | 343 | 332 | 1.03:1 |
CarouselMinimalPerf.default | 414 | 407 | 1.02:1 |
ChatMinimalPerf.default | 641 | 627 | 1.02:1 |
DropdownManyItemsPerf.default | 584 | 572 | 1.02:1 |
GridMinimalPerf.default | 296 | 291 | 1.02:1 |
HeaderMinimalPerf.default | 328 | 320 | 1.02:1 |
ListMinimalPerf.default | 447 | 440 | 1.02:1 |
MenuMinimalPerf.default | 743 | 731 | 1.02:1 |
MenuButtonMinimalPerf.default | 1454 | 1426 | 1.02:1 |
RefMinimalPerf.default | 216 | 212 | 1.02:1 |
TextAreaMinimalPerf.default | 445 | 437 | 1.02:1 |
TreeMinimalPerf.default | 702 | 691 | 1.02:1 |
AnimationMinimalPerf.default | 481 | 474 | 1.01:1 |
AttachmentSlotsPerf.default | 934 | 927 | 1.01:1 |
CheckboxMinimalPerf.default | 2312 | 2292 | 1.01:1 |
DividerMinimalPerf.default | 327 | 325 | 1.01:1 |
ListCommonPerf.default | 547 | 539 | 1.01:1 |
ListWith60ListItems.default | 565 | 558 | 1.01:1 |
LoaderMinimalPerf.default | 601 | 595 | 1.01:1 |
RosterPerf.default | 1027 | 1021 | 1.01:1 |
PortalMinimalPerf.default | 162 | 161 | 1.01:1 |
SliderMinimalPerf.default | 1474 | 1462 | 1.01:1 |
CustomToolbarPrototype.default | 3639 | 3597 | 1.01:1 |
TooltipMinimalPerf.default | 904 | 899 | 1.01:1 |
ButtonOverridesMissPerf.default | 1467 | 1462 | 1:1 |
DialogMinimalPerf.default | 666 | 663 | 1:1 |
EmbedMinimalPerf.default | 3521 | 3534 | 1:1 |
InputMinimalPerf.default | 1122 | 1119 | 1:1 |
ItemLayoutMinimalPerf.default | 1020 | 1015 | 1:1 |
ListNestedPerf.default | 494 | 492 | 1:1 |
PopupMinimalPerf.default | 540 | 539 | 1:1 |
ProviderMergeThemesPerf.default | 1515 | 1511 | 1:1 |
ProviderMinimalPerf.default | 1025 | 1026 | 1:1 |
StatusMinimalPerf.default | 579 | 579 | 1:1 |
TableMinimalPerf.default | 351 | 352 | 1:1 |
TreeWith60ListItems.default | 164 | 164 | 1:1 |
VideoMinimalPerf.default | 536 | 536 | 1:1 |
DatepickerMinimalPerf.default | 4826 | 4895 | 0.99:1 |
DropdownMinimalPerf.default | 2577 | 2604 | 0.99:1 |
LayoutMinimalPerf.default | 314 | 317 | 0.99:1 |
RadioGroupMinimalPerf.default | 390 | 392 | 0.99:1 |
SplitButtonMinimalPerf.default | 3729 | 3776 | 0.99:1 |
IconMinimalPerf.default | 522 | 529 | 0.99:1 |
TableManyItemsPerf.default | 1637 | 1651 | 0.99:1 |
TextMinimalPerf.default | 311 | 314 | 0.99:1 |
ToolbarMinimalPerf.default | 812 | 818 | 0.99:1 |
SkeletonMinimalPerf.default | 305 | 310 | 0.98:1 |
ImageMinimalPerf.default | 329 | 340 | 0.97:1 |
AccordionMinimalPerf.default | 132 | 138 | 0.96:1 |
} | ||
const shouldAddGriffelPreset = pkgJson.dependencies['@griffel/react']; | ||
const extraPresets = shouldAddGriffelPreset ? ['@griffel'] : []; | ||
const config = templates.babelConfig({ extraPresets }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me gusta
@@ -16,7 +16,7 @@ const v8ReferencePackages = { | |||
node: ['codemods'], | |||
}; | |||
const convergedReferencePackages = { | |||
react: ['react-menu'], | |||
react: ['react-provider'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh didnt know we have thing like this, nx migration is run after the plop anyways, so it should be always up to date to latest setup
@@ -34,6 +34,7 @@ | |||
"@types/enzyme-adapter-react-16": "1.0.3", | |||
"@types/react": "16.9.42", | |||
"@types/react-dom": "16.9.10", | |||
"@types/react-test-renderer": "^16.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm working on PR to hoist this to single version policy
PR changes
This PR updates
create-package
script & templates and Nx generator use Griffel packages.Changes that are applied to generator are matching changes in #21360.
Note: This PR does not implement migration from
make-styles
to Griffel.